[SYCL] Add ABI symbols tests against release branch #19860

aelovikov-intel · 2025-08-22T16:54:52Z

Before this PR our approach was to have
sycl_symbols_[linux|windows]-sycl.dump files in trunk that always capture the current state of symbols exported from libsycl.so/sycl.dll We highlighted added/removed symbols separately and considered "added" symbols as being OK while "removed" meant an ABI break. That works fine by is a bit too verbose. Also, not all "removed" are bad. If we've just added a symbol in a previous commit and are reverting it now, the backward ABI compatibility against the last release isn't being broken.

I don't think we promise any backward compatibility between arbitrary builds in the trunk (origin/sycl), only the compatibility against previous official minor releases with the same major version.

This PR adds two more tests that track what we promise better. I've modified our sycl/tools/abi_check.py to allow ignoring "added" symbols and only check against symbols that are being removed. I've also copied sycl_symbols*.dump from the sycl-rel-6_3 branch into the trunk under sycl_symbols*-sycl-rel-6_3.dump names and started running the testing for them in that new mode. Those must never fail, unless we're incrementing the major version or the break is intentional and approved (hence dedicated CODEOWNERS who can approve such changes).

I'm also adding a step to the sycl-nightly.yml workflow to ensure that those copies in trunk match the release branch. That would be handy if we were to cherry-pick some changes that add new symbols to the ongoing release.

.github/CODEOWNERS

Before this PR our approach was to have `sycl_symbols_[linux|windows]-sycl.dump` files in trunk that always capture the current state of symbols exported from `libsycl.so`/`sycl.dll` We highlighted added/removed symbols separately and considered "added" symbols as being OK while "removed" meant an ABI break. That works fine by is a bit too verbose. Also, not all "removed" are bad. If we've just added a symbol in a previous commit and are reverting it now, the backward ABI compatibility against the last release isn't being broken. I don't think we promise any backward compatibility between arbitrary builds in the trunk (`origin/sycl`), only the compatibility against previous official minor releases with the same major version. This PR adds two more tests that track what we promise better. I've modified our `sycl/tools/abi_check.py` to allow ignoring "added" symbols and only check against symbols that are being removed. I've also copied `sycl_symbols*.dump` from the `sycl-rel-6_3` branch into the trunk under `sycl_symbols*-sycl-rel-6_3.dump` names and started running the testing for them in that new mode. Those **must** never fail, unless we're incrementing the major version or the break is intentional and approved (hence dedicated `CODEOWNERS` who can approve such changes). I'm also adding a step to the `sycl-nightly.yml` workflow to ensure that those copies in trunk match the release branch. That would be handy if we were to cherry-pick some changes that add new symbols to the ongoing release.

aelovikov-intel · 2025-08-22T16:59:31Z

.github/workflows/sycl-nightly.yml

+  check_abi_symbols:
+    name: Check ABI symbols tests match release branch
+    runs-on: [Linux, build]
+    if: github.repository == 'intel/llvm'
+    container: ghcr.io/intel/llvm/ubuntu2404_build
+    steps:
+    - uses: actions/checkout@v4
+      with:
+        sparse-checkout: |
+          sycl/test/abi
+    - run: |
+        git fetch origin sycl-rel-6_3
+        git diff --exit-code -I "^# RUN" origin/sycl-rel-6_3:sycl/test/abi/sycl_symbols_linux.dump sycl/test/abi/sycl_symbols_linux-sycl-rel-6_3.dump
+        git diff --exit-code -I "^# RUN" origin/sycl-rel-6_3:sycl/test/abi/sycl_symbols_windows.dump sycl/test/abi/sycl_symbols_windows-sycl-rel-6_3.dump


Verified pass in https://github.com/intel/llvm/actions/runs/17161188384/job/48690624194

Verified "expected/intentional" fail in https://github.com/intel/llvm/actions/runs/17161006785

aelovikov-intel · 2025-08-22T17:03:41Z

Technically, we could remove old "trunk" tests and only use the new ones and reduce the noise. I'm not doing it here for the following reasons:

Live with both new/old approaches in parallel to get more confidence with the new one.
Seeing "added" symbols might still be beneficial, e.g. to catch cases when the addition was unintentional.
We'll need full up-to-date symbols in the future release branches. The easiest way to achieve that is to require trunk to always have them.

uditagarwal97 · 2025-08-25T02:59:00Z

.github/workflows/sycl-nightly.yml

+        git fetch origin sycl-rel-6_3
+        git diff --exit-code -I "^# RUN" origin/sycl-rel-6_3:sycl/test/abi/sycl_symbols_linux.dump sycl/test/abi/sycl_symbols_linux-sycl-rel-6_3.dump
+        git diff --exit-code -I "^# RUN" origin/sycl-rel-6_3:sycl/test/abi/sycl_symbols_windows.dump sycl/test/abi/sycl_symbols_windows-sycl-rel-6_3.dump


NIT: Instead of hardcoding the release version (6.3), can we like have a variable: Something like:

LAST_SYCL_REL_VER="6_3" git fetch origin sycl-rel-$LAST_SYCL_REL_VER git diff --exit-code -I "^# RUN" origin/sycl-rel-$LAST_SYCL_REL_VER:sycl/test/abi/sycl_symbols_linux.dump sycl/test/abi/sycl_symbols_linux-sycl-rel-$LAST_SYCL_REL_VER.dump git diff --exit-code -I "^# RUN" origin/sycl-rel-$LAST_SYCL_REL_VER:sycl/test/abi/sycl_symbols_windows.dump sycl/test/abi/sycl_symbols_windows-sycl-rel-$LAST_SYCL_REL_VER.dump

It would make it easier to update the release version for future releases.

I'd prefer to leave it to the next update. Not exactly sure how it's going to look like. The code-duplication is isolated, and we can remove it if it starts to grow.

uditagarwal97

LGTM

.github/workflows/sycl-nightly.yml

AlexeySachkov · 2025-08-26T08:27:23Z

I don't think we promise any backward compatibility between arbitrary builds in the trunk (origin/sycl), only the compatibility against previous official minor releases with the same major version.

I think that strictly speaking, we do. Otherwise we wouldn't have ABI-breaking windows which are only 3-4 weeks long and would allow breaking ABI during a whole period between latest non-ABI-breaking release X and the upcoming ABI-breaking release X+1

steffenlarsen · 2025-08-26T08:33:42Z

I think that strictly speaking, we do. Otherwise we wouldn't have ABI-breaking windows which are only 3-4 weeks long and would allow breaking ABI during a whole period between latest non-ABI-breaking release X and the upcoming ABI-breaking release X+1

Historically, we've allowed breaks within a short window of the corresponding symbols being introduced, but it is a little bit of a gray area.

vinser52 · 2025-08-26T09:47:35Z

I don't think we promise any backward compatibility between arbitrary builds in the trunk (origin/sycl), only the compatibility against previous official minor releases with the same major version.

I think that strictly speaking, we do. Otherwise we wouldn't have ABI-breaking windows which are only 3-4 weeks long and would allow breaking ABI during a whole period between latest non-ABI-breaking release X and the upcoming ABI-breaking release X+1

What if we need to roll back some commit that introduces a new ABI?

AlexeySachkov · 2025-08-26T09:52:08Z

I don't think we promise any backward compatibility between arbitrary builds in the trunk (origin/sycl), only the compatibility against previous official minor releases with the same major version.

I think that strictly speaking, we do. Otherwise we wouldn't have ABI-breaking windows which are only 3-4 weeks long and would allow breaking ABI during a whole period between latest non-ABI-breaking release X and the upcoming ABI-breaking release X+1

What if we need to roll back some commit that introduces a new ABI?

That is what @steffenlarsen has been referring to as "gray area". I guess the question here is how short the window for a "hotfix" is.

For example, if it all happened within a day, then likely absolutely no one has seen it. If it happened within a few days, then it may already disrupt someone's workflow.

For the record: I'm not trying to say that we shouldn't do "hotfix" ABI-breaks (like reverting accidentally added exported symbol or something) if that is done quickly. But I still do think that we care about preserving ABI between arbitrary builds in the trunk (origin/sycl) when we talk about weeks scale. Those ABI-breaking windows were specifically introduced to limit the time period when ABI is unstable to avoid disruptions in internal testing.

aelovikov-intel · 2025-08-26T15:16:34Z

Those ABI-breaking windows were specifically introduced to limit the time period when ABI is unstable to avoid disruptions in internal testing.

Those windows are about breaking ABI compatibility with the latest official release though, so I'm not sure how applicable that is.

But I still do think that we care about preserving ABI between arbitrary builds in the trunk (origin/sycl) when we talk about weeks scale.

Why? How would it be different for the user if the "break" is because something from yesterday is being reverted/"broken" vs something from a few weeks ago?

aelovikov-intel requested review from a team as code owners August 22, 2025 16:54

aelovikov-intel requested review from gmlueck, sergey-semenov and xtian-github August 22, 2025 16:54

aelovikov-intel had a problem deploying to WindowsCILock August 22, 2025 16:55 — with GitHub Actions Error

aelovikov-intel commented Aug 22, 2025

View reviewed changes

.github/CODEOWNERS Outdated Show resolved Hide resolved

aelovikov-intel had a problem deploying to WindowsCILock August 22, 2025 16:55 — with GitHub Actions Error

aelovikov-intel force-pushed the rel-abi-symbols branch from 0a4001f to 38547e5 Compare August 22, 2025 16:58

aelovikov-intel temporarily deployed to WindowsCILock August 22, 2025 16:59 — with GitHub Actions Inactive

aelovikov-intel commented Aug 22, 2025

View reviewed changes

aelovikov-intel requested a review from vinser52 August 22, 2025 17:03

aelovikov-intel had a problem deploying to WindowsCILock August 22, 2025 17:26 — with GitHub Actions Failure

aelovikov-intel had a problem deploying to WindowsCILock August 22, 2025 19:54 — with GitHub Actions Failure

vinser52 mentioned this pull request Aug 24, 2025

[SYCL] Refactor kernel name based data approach #19117

Merged

uditagarwal97 reviewed Aug 25, 2025

View reviewed changes

uditagarwal97 approved these changes Aug 25, 2025

View reviewed changes

KornevNikita reviewed Aug 25, 2025

View reviewed changes

.github/workflows/sycl-nightly.yml Outdated Show resolved Hide resolved

Update CODEOWNERS and limit fetch depth

901e4a4

aelovikov-intel had a problem deploying to WindowsCILock August 25, 2025 16:26 — with GitHub Actions Error

Align with recent sycl-rel-6_3 update

99aa55e

aelovikov-intel temporarily deployed to WindowsCILock August 25, 2025 16:30 — with GitHub Actions Inactive

vinser52 approved these changes Aug 25, 2025

View reviewed changes

aelovikov-intel had a problem deploying to WindowsCILock August 25, 2025 17:02 — with GitHub Actions Error

aelovikov-intel temporarily deployed to WindowsCILock August 25, 2025 17:02 — with GitHub Actions Inactive

aelovikov-intel temporarily deployed to aws August 25, 2025 17:09 — with GitHub Actions Inactive

aelovikov-intel had a problem deploying to WindowsCILock August 25, 2025 17:26 — with GitHub Actions Failure

aelovikov-intel temporarily deployed to WindowsCILock August 25, 2025 17:26 — with GitHub Actions Inactive

aelovikov-intel had a problem deploying to WindowsCILock August 25, 2025 17:32 — with GitHub Actions Failure

aelovikov-intel temporarily deployed to WindowsCILock August 25, 2025 18:21 — with GitHub Actions Inactive

aelovikov-intel merged commit d6eb331 into sycl Aug 25, 2025
51 of 63 checks passed

aelovikov-intel deleted the rel-abi-symbols branch August 25, 2025 18:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SYCL] Add ABI symbols tests against release branch #19860

[SYCL] Add ABI symbols tests against release branch #19860

Uh oh!

aelovikov-intel commented Aug 22, 2025

Uh oh!

Uh oh!

aelovikov-intel Aug 22, 2025 •

edited

Loading

Uh oh!

aelovikov-intel commented Aug 22, 2025 •

edited

Loading

Uh oh!

uditagarwal97 Aug 25, 2025

Uh oh!

aelovikov-intel Aug 25, 2025

Uh oh!

uditagarwal97 left a comment

Uh oh!

Uh oh!

Uh oh!

AlexeySachkov commented Aug 26, 2025

Uh oh!

steffenlarsen commented Aug 26, 2025

Uh oh!

vinser52 commented Aug 26, 2025

Uh oh!

AlexeySachkov commented Aug 26, 2025 •

edited

Loading

Uh oh!

aelovikov-intel commented Aug 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

[SYCL] Add ABI symbols tests against release branch #19860

[SYCL] Add ABI symbols tests against release branch #19860

Uh oh!

Conversation

aelovikov-intel commented Aug 22, 2025

Uh oh!

Uh oh!

aelovikov-intel Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aelovikov-intel commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uditagarwal97 Aug 25, 2025

Choose a reason for hiding this comment

Uh oh!

aelovikov-intel Aug 25, 2025

Choose a reason for hiding this comment

Uh oh!

uditagarwal97 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlexeySachkov commented Aug 26, 2025

Uh oh!

steffenlarsen commented Aug 26, 2025

Uh oh!

vinser52 commented Aug 26, 2025

Uh oh!

AlexeySachkov commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aelovikov-intel commented Aug 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

aelovikov-intel Aug 22, 2025 •

edited

Loading

aelovikov-intel commented Aug 22, 2025 •

edited

Loading

AlexeySachkov commented Aug 26, 2025 •

edited

Loading